Skip to content

posture-aware Activity orientation and landscape PreviewDisplay framing - #650

Draft
Kimblebee wants to merge 8 commits into
mainfrom
kim/adaptive/preview-display-framing
Draft

Kimblebee wants to merge 8 commits into
mainfrom
kim/adaptive/preview-display-framing

Conversation

@Kimblebee

Copy link
Copy Markdown
Collaborator

Stacked PR: Based on main

Summary

Establishes the orientation and viewfinder-framing foundation for Adaptive Capture across multi-window panes, foldables, and tablets. Replaces MainActivity's static android:screenOrientation="nosensor" manifest lock with a posture-aware runtime policy (NOSENSOR on full-screen < 600dp handhelds; UNSPECIFIED in multi-window or >= 600dp large screens) and upgrades PreviewDisplay to frame wide containers in landscape aspect ratios (4:3 / 16:9), run in a single layout pass without BoxWithConstraints subcomposition, isolate optical coordinates from RTL mirroring, and support keyboard/TalkBack center-focus activation.

Key Changes

  • Activity Orientation & Config-Change Contract (AndroidManifest.xml, MainActivity.kt):
    • Removes static android:screenOrientation="nosensor" and declares android:configChanges="orientation|screenSize|smallestScreenSize|screenLayout|keyboardHidden" so rotations, multi-window resizes, and fold/unfold transitions do not recreate MainActivity (allowing active video recordings to continue across a foldable unfold).
    • Adds resolveRequestedOrientation(isInMultiWindowMode, smallestScreenWidthDp) (600dp threshold) wired across onCreate, addOnMultiWindowModeChangedListener, and onConfigurationChanged.
  • Adaptive Viewfinder Framing & Layout (CaptureScreenComponents.kt, CaptureScaffold.kt):
    • Extracts calculateViewfinderSize(...) to fit AspectRatio.toLandscapeFloat() when useLandscapeAspectRatio && maxWidth > maxHeight (preserving portrait ratios when maxWidth <= maxHeight and returning DpSize.Zero for non-positive bounds).
    • Adds contentAlignment: Alignment = Alignment.TopCenter and useLandscapeAspectRatio: Boolean = false to PreviewDisplay, and enables useLandscapeAspectRatio = true in CaptureScaffold.
    • Replaces BoxWithConstraints (SubcomposeLayout) with a standard Box + single-pass Modifier.layout reporting ViewfinderTopLine, and moves capture-blink alpha + 16.dp corner clipping into draw-phase Modifier.graphicsLayer.
  • Accessibility, Keyboard Focus & Gesture Hygiene (CaptureScreenComponents.kt, strings.xml):
    • Wraps the inner viewfinder subtree in LocalLayoutDirection provides LayoutDirection.Ltr.
    • Adds OnClick ("Focus center"), .focusable, and hardware key activation (Enter, NumPadEnter, DirectionCenter, Spacebar) gated on >= 48.dp interactive bounds, plus accessible Indeterminate loading semantics.

Technical Notes

  • Orientation Policy Invariant: Full-screen compact handhelds (!isInMultiWindowMode && smallestScreenWidthDp < 600) stay on SCREEN_ORIENTATION_NOSENSOR so UI chrome can counter-rotate smoothly via sensor gravity without OS window-rotation animations. Multi-window panes and sw >= 600dp devices use SCREEN_ORIENTATION_UNSPECIFIED to prevent 1:1 OS compatibility letterboxing.
  • Single-Pass Measurement & ViewfinderTopLine: Measuring downstream modifiers (PREVIEW_DISPLAY, onGloballyPositioned for LocalOverlapTargetBounds, transformable, graphicsLayer) with Constraints.fixed(widthPx, heightPx) inside Modifier.layout guarantees overlap bounds and rounded-corner clipping track the exact visible frame while propagating ViewfinderTopLine upward through the outer Box.
  • Optical LTR Isolation: CameraXViewfinder and MutableCoordinateTransformer always emit top-left physical coordinates; forcing LayoutDirection.Ltr inside PreviewDisplay keeps FocusMeteringIndicator's Modifier.offset aligned with physical tap points while preserving ambient RTL outside the viewfinder.

Bugs Fixed Along the Way

  • RTL Focus Reticle Mirroring: Fixed FocusMeteringIndicator rendering horizontally mirrored away from the physical tap point under LayoutDirection.Rtl.
  • Stale Gesture Callback Closures: Fixed pointerInput(onFlipCamera) capturing stale onTapToFocus lambdas (and restarting mid-gesture on onFlipCamera updates) and rememberTransformableState capturing stale onScaleZoom lambdas; migrated to the non-deprecated 4-parameter rememberTransformableState overload.
  • Capture Blink Cancellation Freeze & Reduced-Motion Strobe: Added try/finally around delay(BLINK_TIME) so cancelled blink coroutines cannot leave PreviewDisplay stuck at alpha = 0f, suppressed zero-alpha strobing when LocalDisableAnimations is true, and eliminated per-frame blink recompositions.
  • Zero-Bounds NaN & Loading Spinner Hang: Guarded maxWidth <= 0.dp || maxHeight <= 0.dp against NaN division and switched the loading indicator to a static 0.75f progress arc when LocalDisableAnimations is true while exposing R.string.camera_not_ready semantics.

Not Included Yet

  • Comes later in this series:
    • Live Preview.targetRotation and sensor-aligned ViewPort sync (kim/adaptive/viewfinder-rotation).
    • Orientation lock during active video recording (kim/adaptive/debug-layout-readout, PR add 3-signal rotation, recording lock, and debug layout readout #643).
    • Centered adaptive posture layouts and compact counter-rotation (kim/adaptive/layouts -> kim/adaptive/layout-rotation).
  • Open follow-ups:
    • Hardening recording across unfold: ✨ Recording now continues across unfold on both cameras ✨ ; one non-reproducible front-camera freeze was seen in manual testing, so this path needs broader device testing and automated coverage.
    • Portrait split-screen panes on landscape displays (W < H at windowRotation = 90°/270°): PreviewDisplay currently picks 3:4 vs. 4:3 from pane bounds rather than oriented stream dimensions, center-cropping ~44% (4:3) to ~68% (16:9) of horizontal FOV relative to the saved photo; follow-up will drive framing from SurfaceRequest dimensions/rotation so narrow panes letterbox cleanly (100% WYSIWYG).

Testing

  • ./gradlew :app:testDebugUnitTest --tests "*MainActivityOrientationTest" — 3 unit tests covering compact full-screen (411dp, 599dp), compact multi-window, and large-screen/foldable (600dp, 673dp) orientation policies.
  • ./gradlew :ui:components:capture:testDebugUnitTest --tests "*PreviewDisplayTest" — 15 Robolectric unit & Compose UI tests covering calculateViewfinderSize across phone/tablet/foldable/zero-bound geometries, dynamic 4:3 ↔ 16:9 pillarbox/letterbox overlap bounds, LTR vs. RTL tap-to-focus parity, rememberUpdatedState callback refresh + pinch zoom, keyboard/TalkBack center-focus & < 48dp gating, and frame-by-frame capture blink alpha.
  • ./gradlew :ui:components:capture:connectedDebugAndroidTest -Pandroid.testInstrumentationRunnerArguments.class=com.google.jetpackcamera.ui.components.capture.CaptureScreenComponentsTest — 3 Espresso AccessibilityValidator tests covering dark/light loading states and live SurfaceRequest center-focus semantics.

…viewDisplay

- Extract @VisibleForTesting internal calculateViewfinderSize with non-positive bounds guard and landscape aspect ratio selection when useLandscapeAspectRatio && maxWidth > maxHeight
- Add contentAlignment and useLandscapeAspectRatio trailing default parameters to PreviewDisplay
- Replace BoxWithConstraints with standard Box + Modifier.layout measurement and graphicsLayer draw-phase blink alpha/clip
- Stabilize gesture callbacks with rememberUpdatedState and migrate rememberTransformableState to non-deprecated centroid overload
- Enforce LayoutDirection.Ltr inside PreviewDisplay viewfinder scope so tap-to-focus coordinates and FocusMeteringIndicator never mirror in RTL
- Add PreviewDisplay @previews and Robolectric unit/Compose tests in PreviewDisplayTest
# Conflicts:
#	ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/CaptureScreenComponents.kt
…dow orientation

- Remove static android:screenOrientation="nosensor" from MainActivity in AndroidManifest.xml and declare orientation|screenSize|smallestScreenSize|screenLayout|keyboardHidden configChanges
- Add runtime resolveRequestedOrientation in MainActivity to lock full-screen compact phones (< 600dp) to SCREEN_ORIENTATION_NOSENSOR while using SCREEN_ORIENTATION_UNSPECIFIED in multi-window and on large screens (>= 600dp) to prevent 1:1 OS compatibility letterboxing
- Pass useLandscapeAspectRatio = true to PreviewDisplay in PreviewScreen so wide containers frame the viewfinder in landscape (4:3 / 16:9)
- Add unit tests in MainActivityOrientationTest
…display-framing

# Conflicts:
#	feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewScreen.kt
#	ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/CaptureScreenComponents.kt

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors orientation handling in MainActivity to dynamically adjust based on multi-window mode and screen size, and updates PreviewDisplay to support landscape aspect ratios, custom alignments, and improved accessibility semantics. It also adds comprehensive unit and Robolectric tests. A critical issue was identified in MainActivity where calling isInMultiWindowMode as a default parameter value will cause a crash on devices running below API level 24, requiring an explicit SDK version check.


private fun updateRequestedOrientation(
configuration: Configuration,
multiWindowMode: Boolean = isInMultiWindowMode

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

Calling isInMultiWindowMode directly as a default parameter value will cause a NoSuchMethodError crash on devices running API levels below 24 (Android 7.0), as this property was introduced in API 24. Since the app's minimum SDK version is likely lower (e.g., API 21), this call must be guarded with an API level check.

        multiWindowMode: Boolean = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {
            isInMultiWindowMode
        } else {
            false
        }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant